CORE-2720: Sweep the migrated CSS onto the theme tokens - #143
Open
OpenStaxClaude wants to merge 4 commits into
Open
CORE-2720: Sweep the migrated CSS onto the theme tokens#143OpenStaxClaude wants to merge 4 commits into
OpenStaxClaude wants to merge 4 commits into
Conversation
OpenStaxClaude
added a commit
that referenced
this pull request
Aug 31, 2026
…ur syntax Addresses review on #143. theme.css is now generated rather than hand-written and kept in sync by a test (RoyEJohnson's suggestion). src/theme/themeCss.ts owns the projection from the JS theme into --ox-* custom properties; `npm run generate:theme-css` writes the file; tokens.spec.ts asserts the committed file equals the generator's output. That replaces the three "do these two agree" tests with one that cannot be partially satisfied, and it fixes the stale src/theme/theme.css.spec.ts reference in the header comment, which Copilot caught. The component-CSS colour check now parses declarations instead of grepping for hex. It resolves hex, the functional notations and bare named colours anywhere they appear — shorthands and gradient stops included — while descending into var()/color-mix()/gradients rather than treating them as literals. A translucent colour passes when its opaque channels are a theme value, so shadows keep working without allowlisting each alpha, but a new hue via rgba() is still refused. That lets #33 come off KNOWN_OFF_PALETTE. Also added: a check that every --ox-* a stylesheet reads actually exists. The checker has its own tests now — 28 cases covering what it must flag and what it must leave alone — so the guarantee is tested rather than asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RoyEJohnson
force-pushed
the
CORE-2720-global-css-theme-tokens
branch
from
August 31, 2026 22:40
efdc08b to
3ba2658
Compare
RoyEJohnson
force-pushed
the
CORE-2720-global-css-theme-tokens
branch
from
August 31, 2026 22:52
3ba2658 to
cbf780d
Compare
An error occurred while trying to automatically change base from
CORE-2710-compose-render-props-style
to
main
September 8, 2026 18:55
OpenStaxClaude
added a commit
that referenced
this pull request
Sep 8, 2026
…ur syntax Addresses review on #143. theme.css is now generated rather than hand-written and kept in sync by a test (RoyEJohnson's suggestion). src/theme/themeCss.ts owns the projection from the JS theme into --ox-* custom properties; `npm run generate:theme-css` writes the file; tokens.spec.ts asserts the committed file equals the generator's output. That replaces the three "do these two agree" tests with one that cannot be partially satisfied, and it fixes the stale src/theme/theme.css.spec.ts reference in the header comment, which Copilot caught. The component-CSS colour check now parses declarations instead of grepping for hex. It resolves hex, the functional notations and bare named colours anywhere they appear — shorthands and gradient stops included — while descending into var()/color-mix()/gradients rather than treating them as literals. A translucent colour passes when its opaque channels are a theme value, so shadows keep working without allowlisting each alpha, but a new hue via rgba() is still refused. That lets #33 come off KNOWN_OFF_PALETTE. Also added: a check that every --ox-* a stylesheet reads actually exists. The checker has its own tests now — 28 cases covering what it must flag and what it must leave alone — so the guarantee is tested rather than asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OpenStaxClaude
force-pushed
the
CORE-2720-global-css-theme-tokens
branch
from
September 8, 2026 19:03
f9cf833 to
5f4d6f9
Compare
OpenStaxClaude
changed the base branch from
CORE-2710-compose-render-props-style
to
main
September 8, 2026 19:04
OpenStaxClaude
added a commit
that referenced
this pull request
Sep 8, 2026
Stacked on #143, so these three components use the :root token file rather than binding theme values inline per component -- the same sweep #143 applied to everything already migrated. - SidebarNav/ButtonNav/Pagination CSS now reads var(--component-hook, var(--ox-token)), and the inline CSSPropertiesWithVariables objects are gone. - The sidebar backdrop z-indexes stop being literals. That caveat only existed because component-scoped custom properties inherit down the tree and the backdrop rules target siblings of the nav; :root tokens reach siblings fine, so these are now calc(var(--ox-z-index-navbar) + 1) and calc(var(--ox-z-index-sidebar) - 1). - #959595 on the toggle button border is genuinely off-palette, carried over verbatim from the styled-components original, so it goes in KNOWN_OFF_PALETTE with a reason rather than being snapped to neutralMedium, which would be a visual change rather than a refactor. The three "binds theme values as custom properties" tests are dropped: there is no inline binding left to assert, and tokens.spec.ts now checks the theme wiring centrally. Replaced where a public contract survives -- that consumers can still override through the documented --sidebar-nav-* / --pagination-* hooks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OpenStaxClaude
added a commit
that referenced
this pull request
Sep 8, 2026
Follows the CORE-2720 (#143) sweep, which this branch is now stacked on. The two new stylesheets repeated eleven palette hexes as var() fallbacks; they now read the --ox-* tokens instead, e.g. color: var(--help-menu-button-color, var(--ox-color-gray)); The override hooks are unchanged -- only their defaults moved from JavaScript to the CSS side, so the components no longer bind static custom properties inline. That means style is no longer destructured in ProfileMenuButton, ProfileMenuItem, HelpMenuButton or HelpMenuItem: it passes through in ...props and react-aria handles both the object and render-callback forms itself. Two consequences worth naming: - The CORE-2710 (#137) dependency is gone rather than deferred. The bug it guards against was a wrapper overwriting the caller's style, which these wrappers no longer do, so the menu items need nothing from #137. Same reasoning as the note #143 leaves on NavBarMenuItem. - iframeWrapperStyle and putAwayStyle are gone; the iframe wrapper and the put-away bar take their colours from HelpMenu.css. className composition stays -- that one is a real bug fix, not a default. The specs that asserted the inline defaults now assert what matters instead: the caller's style reaches the element in both forms, and the override hook still wins. Defaults are covered centrally by src/theme/tokens.spec.ts, which also fails on any colour literal that duplicates a theme value -- both new stylesheets pass it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces repo-wide build/test enforcement plus broad styling and snapshot churn that should be validated end-to-end by a human reviewer in a real consumer/bundler context.
Review details
- Files reviewed: 64/64 changed files
- Comments generated: 0 new
- Review effort level: Lite
Establishes one place a theme value is written and referenced from CSS, with
a test that fails if the two disagree. The sweep of the already-migrated
stylesheets onto it follows separately.
src/theme/theme.css holds a single :root block. Colour tokens are the
kebab-case palette key (palette.neutralLighter -> --ox-color-neutral-lighter)
plus --ox-color-link, --ox-color-link-hover, --ox-z-index-* and
--ox-padding-navbar-*. The --ox- prefix avoids collisions with a consuming
app's own variables.
The file is generated, not hand-written. themeCss.ts owns the projection and
npm run generate:theme-css writes it; build.bash runs it as its first step,
before either tsc pass and before the rsync, and publish.bash inherits that
via build:clean, so a published package cannot ship a stale file. It is
committed as well as generated because jest and ladle read src/ directly and
CI runs lint/test rather than build. Deliberately not hooked into pretest —
regenerating before the suite would make the freshness check pass vacuously.
Adds the four button variant colours to the palette, which theme/buttons.ts
had been holding as bare string literals with nothing recording that they
are hover/active variants of orange and darkGray. Purely additive.
Enforcement, in tokens.spec.ts, on top of the CORE-2736 engine:
1. The committed theme.css is exactly what the generator produces. One
equality, so a missing token, an orphan token and a stale value all
fail the same way.
2. No component stylesheet writes a colour literal that duplicates a
theme value.
3. No component stylesheet introduces a colour that is neither a theme
value nor on the KNOWN_OFF_PALETTE allowlist, each entry with a reason.
4. No component stylesheet reads an --ox-* token that does not exist,
which would otherwise fall through to its fallback silently.
Check 2 cannot pass yet — 16 stylesheets migrated before the tokens existed
still carry hand-copied literals. PENDING_SWEEP names them, and is asserted
to be exactly the failing set so it cannot rot in either direction: dropping
a name without sweeping the file fails, and sweeping a file without dropping
its name fails too. The list reaches empty in the sweep PR and goes away
with the assertion.
No CSS @import: build.bash rsyncs CSS 1:1 with no bundler, so an @import
would depend on each consumer's resolver. Component .tsx files import
theme.css alongside their own stylesheet instead. Consumers need do nothing.
Split out of #143. Stacked on #149.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The "what CI enforces" list read as though the duplicate-literal check covered every stylesheet, which it will not until the sweep lands. Says which files are exempt, why the list cannot drift, and that new stylesheets are not to be added to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces the hand-copied literals in the 22 stylesheets migrated before the tokens existed with token references, and empties PENDING_SWEEP so the duplicate-literal check now covers every stylesheet in src/. The --component-* override hooks are unchanged. Only their defaults moved, from a JS inline style to the CSS side: var(--tabs-active-border-color, var(--ox-color-dark-green)) Static bindings dropped from Tooltip, ButtonBar, Tabs, Toast, ToastContainer, NavBar, NavBarMenuButtons, Radio, Checkbox, TreeCheckbox and ButtonLink — 44 of them across 19 components, each existing only to push a static palette colour through an inline style on every render. Dynamic bindings stay in JS: the button and checkbox variant lookups, navbar height / maxWidth / justifyContent, disabled opacity, and the dropdown caret colour. Behaviour note. Removing an inline default changes cascade precedence. A consumer setting e.g. --tabs-border-color on an ancestor element via their own stylesheet used to lose to the inline default; now it applies. Passing the variable through the component's own style prop — the documented path, and the one the specs cover — is unaffected. Specs that asserted the inline default were rewritten to assert the thing that matters, that a caller override still works. tokens.spec.ts now guards the defaults themselves. Fixes a latent bug in passing: --button-shadow is shared by Button.css and DropdownMenu.css and the JS binds palette.black for all three variants, but Button's fallback had drifted to #424242 while DropdownMenu's was #000000. Unreachable today because the JS always sets it, so no visual change — but the two would have rendered the same variant differently if it were ever reached. Genuinely off-palette values (#ccc, #ddd) are left alone on the allowlist with a reason rather than snapped to the nearest palette entry. That is a design decision, not a refactor. Includes the Button.css and DropdownMenu.css half of Roy's cbf780d, which renames the button variant tokens. It cannot be a separate commit on this branch: it edits token references inside already-swept CSS, so it has nothing to apply to until this commit lands. The theme half is preserved as his own commit on #150. Roy's 463bb86 — dropping a needless style-prop rename in ButtonBar — is likewise folded in rather than reverted and reapplied. Split out of the original #143. Stacked on #150; the resulting tree is byte-identical to the reviewed #143 head (7f8bdfb content), so the split carries no change of its own. Co-authored-by: Roy Johnson <roy.e.johnson@rice.edu> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 10, 2026
OpenStaxClaude
force-pushed
the
CORE-2720-global-css-theme-tokens
branch
from
September 10, 2026 18:59
4424b6f to
1a50ffa
Compare
OpenStaxClaude
removed this pull request from stack #147
September 10, 2026 18:59
OpenStaxClaude
added a commit
that referenced
this pull request
Sep 10, 2026
Follows the CORE-2720 (#143) sweep, which this branch is now stacked on. The two new stylesheets repeated eleven palette hexes as var() fallbacks; they now read the --ox-* tokens instead, e.g. color: var(--help-menu-button-color, var(--ox-color-gray)); The override hooks are unchanged -- only their defaults moved from JavaScript to the CSS side, so the components no longer bind static custom properties inline. That means style is no longer destructured in ProfileMenuButton, ProfileMenuItem, HelpMenuButton or HelpMenuItem: it passes through in ...props and react-aria handles both the object and render-callback forms itself. Two consequences worth naming: - The CORE-2710 (#137) dependency is gone rather than deferred. The bug it guards against was a wrapper overwriting the caller's style, which these wrappers no longer do, so the menu items need nothing from #137. Same reasoning as the note #143 leaves on NavBarMenuItem. - iframeWrapperStyle and putAwayStyle are gone; the iframe wrapper and the put-away bar take their colours from HelpMenu.css. className composition stays -- that one is a real bug fix, not a default. The specs that asserted the inline defaults now assert what matters instead: the caller's style reaches the element in both forms, and the override hook still wins. Defaults are covered centrally by src/theme/tokens.spec.ts, which also fails on any colour literal that duplicates a theme value -- both new stylesheets pass it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OpenStaxClaude
added a commit
that referenced
this pull request
Sep 10, 2026
Stacked on #143, so these three components use the :root token file rather than binding theme values inline per component -- the same sweep #143 applied to everything already migrated. - SidebarNav/ButtonNav/Pagination CSS now reads var(--component-hook, var(--ox-token)), and the inline CSSPropertiesWithVariables objects are gone. - The sidebar backdrop z-indexes stop being literals. That caveat only existed because component-scoped custom properties inherit down the tree and the backdrop rules target siblings of the nav; :root tokens reach siblings fine, so these are now calc(var(--ox-z-index-navbar) + 1) and calc(var(--ox-z-index-sidebar) - 1). - #959595 on the toggle button border is genuinely off-palette, carried over verbatim from the styled-components original, so it goes in KNOWN_OFF_PALETTE with a reason rather than being snapped to neutralMedium, which would be a visual change rather than a refactor. The three "binds theme values as custom properties" tests are dropped: there is no inline binding left to assert, and tokens.spec.ts now checks the theme wiring centrally. Replaced where a public contract survives -- that consumers can still override through the documented --sidebar-nav-* / --pagination-* hooks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jira: CORE-2720
3 of 3 — split of this PR at Roy's request.
main→ #149 → #150 → #143.Was +1606/-410 across 64 files. Now +180/-449 across 55, because the two layers underneath it moved out: the colour audit engine to #149 and the token file plus its enforcement to #150. What is left is the part that can actually change a rendered pixel.
What this does
Replaces the hand-copied literals in the 22 stylesheets migrated before the tokens existed with token references, and empties
PENDING_SWEEPso #150's duplicate-literal check now covers every stylesheet insrc/.The
--component-*override hooks are unchanged. Only their defaults moved, from a JS inline style to the CSS side:Static bindings dropped from Tooltip, ButtonBar, Tabs, Toast, ToastContainer, NavBar, NavBarMenuButtons, Radio, Checkbox, TreeCheckbox and ButtonLink — 44 of them across 19 components, each existing only to push a static palette colour through an inline style on every render. Dynamic bindings stay in JS: the button and checkbox variant lookups, navbar height / maxWidth / justifyContent, disabled opacity, the dropdown caret colour.
The one behaviour change
Removing an inline default changes cascade precedence. A consumer setting e.g.
--tabs-border-coloron an ancestor element via their own stylesheet used to lose to the inline default; now it applies. Passing the variable through the component's ownstyleprop — the documented path, and the one the specs cover — is unaffected.Specs that asserted the inline default were rewritten to assert the thing that matters, that a caller override still works. #150's
tokens.spec.tsguards the defaults themselves.Latent bug fixed in passing
--button-shadowis shared byButton.cssandDropdownMenu.cssand the JS bindspalette.blackfor all three variants, but Button's fallback had drifted to#424242while DropdownMenu's was#000000. Unreachable today because the JS always sets it, so no visual change — but the two would have rendered the same variant differently if it were ever reached.Genuinely off-palette values (
#ccc,#ddd) are left alone on the allowlist with a reason rather than snapped to the nearest palette entry. That is a design decision, not a refactor.Verification
git diffbetween this branch's tree and the pre-split CORE-2720: Sweep the migrated CSS onto the theme tokens #143 head (4424b6fd) is empty, so the split carries no change of its own and the evidence below still holds.styleattribute losing a static custom property, with no DOM structure change (checked by filtering the whole snapshot diff for non-style lines).Provenance
Includes the
Button.css/DropdownMenu.csshalf of Roy'scbf780d. It cannot be a separate commit here: it edits token references inside already-swept CSS, so it has nothing to apply to until this commit lands. The theme half is preserved as his own commit on #150. Roy's463bb86d(dropping a needless style-prop rename in ButtonBar) is likewise folded in rather than reverted and reapplied.Pre-split SHA for the record:
4424b6fd.🤖 Generated with Claude Code